-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Subaru: Crosstrek Hybrid dashcam mode #25378
Conversation
@martinl got a test route for me? |
f3c6786
to
ca66665
Compare
<previouslyLoaded_Datafiles> | ||
<fileInfo filename="../../../../threepilot/tools/plotjuggler/tmp03i8v25j.rlog" prefix=""> | ||
<selected_datasources value=""/> | ||
<plugin ID="DataLoad Rlog"/> | ||
</fileInfo> | ||
</previouslyLoaded_Datafiles> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<previouslyLoaded_Datafiles> | |
<fileInfo filename="../../../../threepilot/tools/plotjuggler/tmp03i8v25j.rlog" prefix=""> | |
<selected_datasources value=""/> | |
<plugin ID="DataLoad Rlog"/> | |
</fileInfo> | |
</previouslyLoaded_Datafiles> |
It appears there are no similar messages on bus 0 to the cruise active status. Most common bits are quite a bit off: hex(msg)='0x361', bit_mismatches=981 of 3720 (26.37%), byt_idx=1, bit_idx=2
hex(msg)='0x1f9', bit_mismatches=1476 of 6009 (24.56%), byt_idx=5, bit_idx=7
hex(msg)='0x1f9', bit_mismatches=1002 of 6009 (16.67%), byt_idx=5, bit_idx=6
hex(msg)='0x244', bit_mismatches=1589 of 10190 (15.59%), byt_idx=4, bit_idx=3
Searched bus: 0 On bus 2, it gets a bit interesting. There's of course the bit on 0x321 which is what I'm comparing ( ex(msg)='0x321', bit_mismatches=864 of 2423 (35.66%), byt_idx=5, bit_idx=2
hex(msg)='0x220', bit_mismatches=56 of 4800 (1.17%), byt_idx=0, bit_idx=0
hex(msg)='0x220', bit_mismatches=56 of 4800 (1.17%), byt_idx=4, bit_idx=0
hex(msg)='0x321', bit_mismatches=9 of 2423 (0.37%), byt_idx=4, bit_idx=3
Searched bus: 2 However I checked both signals on older ICE cars, and they proved to be inaccurate, specifically:
There's no other similar signals to |
This is also missing the hybrid signals for gas and brake: https://github.com/ClockeNessMnstr/openpilot/blob/crosstrek-pid/selfdrive/car/subaru/carstate.py#L23
|
There is also ES_Status (0x222) Cruise_Activated, I'll check if it behaves differently |
On Crosstrek ICE, ES_Status Cruise_Activated is identical to CruiseControl Cruise_Activated signal, I tested with 3 cases above - stopping behind lead car / hold (standstill), disengage using brake while in standstill, gas press while engaged |
I checked Crosstrek Hybrid dbc and it does not have ES_Status message. I'll find the signal once I find someone who can provide route for Crosstrek Hybrid |
crosstrek hybrid route: 31d10447d8c7dc19|2023-08-17--07-30-31--0 |
I'll just merge the hybrid cars behind dashcam, we still need a good cruise_activated bit. Would be helpful to get a forester hybrid route so I can see if they are the same |
Can you merge #24770 as dashcam after this? Make sure to leave a comment in the code at least of what is required to un-dashcam |
@@ -3,6 +3,7 @@ | |||
from openpilot.selfdrive.car import apply_driver_steer_torque_limits | |||
from openpilot.selfdrive.car.subaru import subarucan | |||
from openpilot.selfdrive.car.subaru.values import DBC, GLOBAL_GEN2, PREGLOBAL_CARS, CanBus, CarControllerParams, SubaruFlags | |||
from selfdrive.car.subaru.values import HYBRID_CARS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we already import from values above
if self.car_fingerprint not in HYBRID_CARS: # Hybrid cars don't have es_distance, need a replacement | ||
# 8 is known AEB, there are a few other values related to AEB we ignore | ||
ret.stockAeb = (cp_es_distance.vl["ES_Brake"]["AEB_Status"] == 8) and \ | ||
(cp_es_distance.vl["ES_Brake"]["Brake_Pressure"] != 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spacing
self.es_distance_msg = copy.copy(cp_es_distance.vl["ES_Distance"]) | ||
|
||
self.es_status_msg = copy.copy(cp_es_status.vl["ES_Status"]) | ||
self.cruise_control_msg = copy.copy(cp_cruise.vl["CruiseControl"]) | ||
|
||
if self.car_fingerprint not in HYBRID_CARS: | ||
self.es_distance_msg = copy.copy(cp_es_distance.vl["ES_Distance"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like we can simplify this so we don't need to copy twice?
Make sure to request a final review before merging |
No description provided.